refactor(ci/pr-review): use workflow_call instead of workflow_run event
#16891
Conversation
github-actions
bot
added
the
system
| if: ${{ env.GIT_DIFF_CONTENT }} | ||
| run: | | ||
| # Save the raw diff blob and store that inside the ./build/ | ||
| # Download the raw diff blob and store that inside the build | ||
| # directory. | ||
| # The purpose of this is for the PR Review Companion to later | ||
| # be able to use this raw diff file for the benefit of analyzing. | ||
| wget https://github.com/${{ github.repository }}/compare/${{ env.BASE_SHA }}...${{ env.HEAD_SHA }}.diff -O ${{ env.BUILD_OUT_ROOT }}/DIFF |
There was a problem hiding this comment.
Merge the steps, align with the mdn/content's workflow
As the `pr-test` workflow is now a required ci test. This should be triggered in any PRs.
|
@bsmth would you mind to take a look, thanks 🤗 |
There was a problem hiding this comment.
Looks good, just diffed with https://www.diffchecker.com/EzXNI3G4/
There was a problem hiding this comment.
Slight differences here: https://www.diffchecker.com/bU2WdBmN/, you may want to grab the latest version from main as there were some bugfixes on "non-markdown only" changed files, e.g.:
if: ${{ env.GIT_DIFF_CONTENT }} || ${{ env.GIT_DIFF_FILES }
There was a problem hiding this comment.
Which reminds me that these workflows should probably (eventually) live in https://github.com/mdn/workflows as they will diverge over time.
bsmth
dismissed
their stale review
Some deviations from the content repo that would be nice to have covered before we merge
|
Check the workflow run here: https://github.com/mdn/translated-content/actions/runs/7142643657/job/19452360341?pr=17304 It works well. Thank you @bsmth ❤️ |
|
Well done 💯 |
Description
This PR mainly switch to
workflow_callinstead ofworkflow_runevent to prevent calling "PR review companion" when not necessary.To achieve this, and take this opportunity to improve the code, the following changes were made:
pull_request_targetevent instead ofpull_request, with the former event, theGITHUB_TOKENhas write permissions and the jobs could access GitHub secrets (we are using secrets to store the AK, SK).read-allto avoid accidental writes to the repo.ifcondition to skip unnecessary runs,Set the permissions of "review" job to
write-all, so the deployer could create comment in PRs.Add
secrets: inheritto inherit the secrets from the parent workflow so the reusable workflow could access all secrets. (tested in: remove some lines for test yin1999/content#8, https://github.com/yin1999/content/actions/runs/6769815012/job/18397036827?pr=8#step:10:34)actions/download-artifactinstead of js scripts, as the shared "PR review companion" workflow is a spread job in "PR test" now, we could use this action to download and unzip artifacts (simplifies the steps).pr-testworkflow, as this workflow is a required ci test (we should not skip it).Security statement
Because the
GITHUB_TOKENgenerated in workflows that triggered bypull_request_targetevent has write permissions, we need to avoid malicious damage to the repo by the PR author.read-all(as we still need theGITHUB_TOKENto access some resouces).pull_request_targetevent would trigger the workflow runs with themainversion of workflows (including the shared workflows - "PR review companion"). This has been tested by Onkar - Test workflow_call yin1999/content#6, and has been documented by GitHub Docs:pull_request_targetevent could also access GitHub secrets. But GitHub Actions can only read a secret if you explicitly include the secret in a workflow.Related issues and pull requests
Reflect: mdn/content#30064
Tested in my fork: yin1999#29